Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix raid benchmark panics #2888

Merged
merged 2 commits into from
Apr 8, 2023
Merged

Fix raid benchmark panics #2888

merged 2 commits into from
Apr 8, 2023

Conversation

zku
Copy link
Contributor

@zku zku commented Apr 8, 2023

The current benchmark test RaidBenchmark uses TBC item presets, but these do not exist in the database anymore and cause panics (item not found in database). Some enchant IDs were also wrong (using the item ID instead of the enchant ID). Even with WotLK items that exist in the database, the test panics because the enhancement shaman's gear is nil:

λ go test --tags=with_db -benchmem -run=^$ -bench ^BenchmarkSimulate$ -benchmem -memprofile memprofile.out -cpuprofile profile.out github.com/wowsims/wotlk/sim
--- FAIL: BenchmarkSimulate
    test_utils.go:232: RaidBenchmark() at iteration 0 failed: runtime error: invalid memory address or nil pointer dereference
        Stack Trace:
        goroutine 50 [running]:
        panic({0xc44cc0, 0x1c31fc0})
        github.com/wowsims/wotlk/sim/core.ProtoToEquipmentSpec(...)
        github.com/wowsims/wotlk/sim/core.ProtoToEquipment(_)
        github.com/wowsims/wotlk/sim/core.NewCharacter(_, _, _)
        github.com/wowsims/wotlk/sim/core.NewAgent(0xc9cf60?, 0xc000068fa0?, 0xc000850000)
        github.com/wowsims/wotlk/sim/core.NewParty(0xc00007c140, 0x2, 0xc0015a6000)

This PR fixes both panics and makes the benchmark run through the whole sim again. Specifically, the PR

  • updates items to use WotLK phase2 bis presets (instead of TBC)
  • gives the enhancement shaman some gear (also WotLK phase2 bis preset)

@zku zku merged commit 50f3be8 into wowsims:master Apr 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant